Skip to content

[test] Remove dead nil guards and unreachable else branch in logger/jsonl_logger.go - #9714

Merged
lpcox merged 1 commit into
mainfrom
test-coverage/remove-dead-nil-guards-jsonl-logger-206e89da73c72731
Jul 20, 2026
Merged

[test] Remove dead nil guards and unreachable else branch in logger/jsonl_logger.go#9714
lpcox merged 1 commit into
mainfrom
test-coverage/remove-dead-nil-guards-jsonl-logger-206e89da73c72731

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Code Quality Improvement: Dead Code Removal in jsonl_logger.go

What Changed

  • Package: internal/logger
  • File: internal/logger/jsonl_logger.go
  • Functions: LogDifcFilteredItem, LogUnrecognizedEndpointPassthrough
  • Coverage Before: LogDifcFilteredItem 88.9%, LogUnrecognizedEndpointPassthrough 75.0%
  • Coverage After: Both 100%

Why These Changes?

The withGlobalLogger helper (in global_helpers.go:127) already guarantees a non-nil logger before invoking any callback:

func withGlobalLogger[T closableLogger](mu *sync.RWMutex, logger *T, fn func(T)) {
    mu.RLock()
    defer mu.RUnlock()
    if *logger != nil {   // nil check happens HERE
        fn(*logger)       // fn is only called when logger is non-nil
    }
}

The if logger == nil { return } guards inside the LogDifcFilteredItem and LogUnrecognizedEndpointPassthrough callbacks were therefore unreachable dead code — the callback is only ever called with a guaranteed non-nil logger.

This is inconsistent with all other withGlobalLogger callbacks in the codebase (rpc_logger.go, observed_url_domains_logger.go) which do not have redundant nil guards.

Additional Simplification

LogUnrecognizedEndpointPassthrough also had an if/else pattern for json.Marshal where the else branch was unreachable: JSONLUnrecognizedEndpointPassthrough has only string fields, so marshaling cannot fail. Replaced with b, _ := json.Marshal(entry), matching the pattern in rpc_logger.go.

Tests

Existing tests in jsonl_logger_test.go continue to pass and cover both functions:

  • TestLogDifcFilteredItem_NoLogger — verifies no panic when logger is uninitialized
  • TestLogDifcFilteredItem_WritesAuditEntryToJSONL — verifies correct JSONL output
  • TestLogUnrecognizedEndpointPassthrough_NoLogger — verifies no panic when logger is uninitialized
  • TestLogUnrecognizedEndpointPassthrough_WritesCorrectFields — verifies correct JSONL output

Coverage Report

Before: LogDifcFilteredItem 88.9%, LogUnrecognizedEndpointPassthrough 75.0%, Total 99.5%
After:  LogDifcFilteredItem 100.0%, LogUnrecognizedEndpointPassthrough 100.0%, Total 99.7%
Improvement: +0.2%

Generated by Test Coverage Improver

Warning

Firewall blocked 5 domains

The following domains were blocked by the firewall during workflow execution:

  • awmgmcpg
  • goproxy.cn
  • goproxy.io
  • proxy.golang.org
  • releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"
    - "goproxy.cn"
    - "goproxy.io"
    - "proxy.golang.org"
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Generated by Test Coverage Improver · 637.4 AIC · ⊞ 6.9K ·

The withGlobalLogger helper already guarantees a non-nil logger before
invoking any callback (see global_helpers.go:127). The nil guard checks
inside the LogDifcFilteredItem and LogUnrecognizedEndpointPassthrough
callbacks were therefore dead code — they could never be reached.

Also simplify json.Marshal for JSONLUnrecognizedEndpointPassthrough: the
struct has only string fields, so marshaling cannot fail. Replace the
if/else error pattern with b, _ := json.Marshal(entry) to match similar
patterns elsewhere (e.g. rpc_logger.go).

These changes are consistent with all other withGlobalLogger callbacks in
the package (rpc_logger.go, observed_url_domains_logger.go) which do not
include redundant nil guards.

Coverage improvement: LogDifcFilteredItem 88.9% → 100%,
LogUnrecognizedEndpointPassthrough 75% → 100%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox
lpcox marked this pull request as ready for review July 20, 2026 17:21
Copilot AI review requested due to automatic review settings July 20, 2026 17:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes unreachable defensive branches from JSONL logging callbacks.

Changes:

  • Removes redundant nil guards already enforced by withGlobalLogger.
  • Simplifies marshaling of a string-only audit entry.
Show a summary per file
File Description
internal/logger/jsonl_logger.go Removes dead branches from two logging paths.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Medium

@lpcox
lpcox merged commit 274a6c2 into main Jul 20, 2026
23 checks passed
@lpcox
lpcox deleted the test-coverage/remove-dead-nil-guards-jsonl-logger-206e89da73c72731 branch July 20, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants